[Property Editor] Emphasize properties the user has set in their code #8637
[Property Editor] Emphasize properties the user has set in their code #8637elliette merged 18 commits intoflutter:masterfrom
Conversation
|
This makes the non-set properties look disabled IMO. Is this something we could put on our list to get UX input on? what about another badge "explicit" or "set" instead of graying out some of the properties? |
We already have 2 potential badges (isDefault and isRequired), and I think we will need to change the UI to fit 3. I've tweaked how we are determining the background color, and added a bunch of different screenshots for various VS Code themes. Let me know what you think - mostly I want this in a nice-ish state for the demo, but I agree this is something we should follow up on with UX. Note: I haven't added comments and tests yet, will add those if we think we should go ahead with these colors for now. |
|
What about something more subtle like this? |
|
Another affordance to consider if we want to reduce the number of badges like "default". We could use an asterisk |
Oh I like this option! Changed to that and updated the screenshots. |
| import '../../../shared/primitives/utils.dart'; | ||
| import 'property_editor_controller.dart'; | ||
|
|
||
| class PropertyEditorView extends StatelessWidget { |
There was a problem hiding this comment.
this widget doesn't provide much value to abstract now. Can we add the _PropertiesList directly where PropertyEditorView is currently used? That or make PropertyEditorView what _PropertiesList currently is (effectively deleting _PropertiesList and having PropertyEditorView contain the body of the current _PropertiesList)
There was a problem hiding this comment.
Yes, we can! Though I'm planning on adding more to the PropertyEditorView soon (for example, the widget name) so will probably end up adding it back in later. Do you still think I should delete it for now?
There was a problem hiding this comment.
I can add a TODO for the parts that are coming soon
| ), | ||
| child: Row( | ||
| crossAxisAlignment: CrossAxisAlignment.start, | ||
| children: [ |
There was a problem hiding this comment.
nit: it might be simpler to just add a Container to this row directly:
Row(
children: [
Container(
width: _hasArgIndicatorWidth,
color: argument.hasArgument ? theme.colorScheme.primary : Colors.transparent,
),
...
],
),
There was a problem hiding this comment.
Hmm I tried that and was struggling to get it the proper height which is why I switched to border instead
elliette
added
the
autosubmit
|
auto label is removed for flutter/devtools/8637, due to - The status or check suite main has failed. Please fix the issues identified (or deflake) before re-applying this label. |
auto-submit
Bot
removed
the
autosubmit
This reverts commit 3ca9942.
2 participants
Fixes #8653
Work towards #1948
We would like to highlight the properties that the user has set in their code so that its visually clear what they have set.
I swapped the
requiredlabel with asetlabel, and moved required to the input indicator (following the Material 3 guidance):Screenshots of various themes (NOTE: before
requiredlabel was changed):